Skip to content

Conversation

rmatif
Copy link
Collaborator

@rmatif rmatif commented Aug 2, 2025

Synchronize the command queue via clFinish() before gathering profiling data to ensure all commands are complete.

This fixes a crash when profiling is enabled while launching llama-bench on Adreno 830

@rmatif rmatif requested review from lhez and max-krasnyansky August 2, 2025 20:39
@github-actions github-actions bot added ggml changes relating to the ggml tensor library for machine learning OpenCL Issues specific to the OpenCL backend labels Aug 2, 2025
@lhez
Copy link
Collaborator

lhez commented Aug 3, 2025

Does it always crash with llama-bench?

Waiting for each event already makes sure the corresponding kernel is complete. I think it's more likely events are being released twice. This happened with llama-mtmd. I will also take a closer look into this crash.

@rmatif
Copy link
Collaborator Author

rmatif commented Aug 3, 2025

Does it always crash with llama-bench?

Waiting for each event already makes sure the corresponding kernel is complete. I think it's more likely events are being released twice. This happened with llama-mtmd. I will also take a closer look into this crash.

Yes, it was crashing consistently. I believe the crash occurs at clWaitForEvents, not clReleaseEvent. The issue only happens during teardown, so it might be a driver issue

@lhez
Copy link
Collaborator

lhez commented Aug 4, 2025

llama-bench creates a new context for each new params combination instance. profiling_info needs to be cleared in free() so that it will start with clean slate and previously freed events will not be waited on again. We don't need to remove clWaitForEvents and clReleaseEvent or add clFinish.

The following should fix llama-bench crash,

diff --git a/ggml/src/ggml-opencl/ggml-opencl.cpp b/ggml/src/ggml-opencl/ggml-opencl.cpp
index c9316eb7..ceb797a9 100644
--- a/ggml/src/ggml-opencl/ggml-opencl.cpp
+++ b/ggml/src/ggml-opencl/ggml-opencl.cpp
@@ -600,6 +600,7 @@ struct ggml_backend_opencl_context {
         if (ref_count == 0) {
 #ifdef GGML_OPENCL_PROFILING
             write_profiling_info();
+            profiling_info.clear();
 #endif
         }
     }

@rmatif
Copy link
Collaborator Author

rmatif commented Aug 4, 2025

The following should fix llama-bench crash,

diff --git a/ggml/src/ggml-opencl/ggml-opencl.cpp b/ggml/src/ggml-opencl/ggml-opencl.cpp
index c9316eb7..ceb797a9 100644
--- a/ggml/src/ggml-opencl/ggml-opencl.cpp
+++ b/ggml/src/ggml-opencl/ggml-opencl.cpp
@@ -600,6 +600,7 @@ struct ggml_backend_opencl_context {
         if (ref_count == 0) {
 #ifdef GGML_OPENCL_PROFILING
             write_profiling_info();
+            profiling_info.clear();
 #endif
         }
     }

Alright, I just tested it and it works. Closing this PR and opening a new one since I was trying to merge into the wrong branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ggml changes relating to the ggml tensor library for machine learning OpenCL Issues specific to the OpenCL backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants